feat: add check to verify backend connection in readiness probe#919
feat: add check to verify backend connection in readiness probe#919nancynh wants to merge 1 commit into
Conversation
nancynh
force-pushed
the
backend-check
branch
6 times, most recently
from
85ba4ae to
fec9183
Compare
|
|
||
| if c.withBackendCheck { | ||
| var lastErr error | ||
| for { |
There was a problem hiding this comment.
We can skip the for loop as Cloud Run (or K8s) will continue to invoke this endpoint at a regular interval. See https://docs.cloud.google.com/run/docs/configuring/healthchecks#readiness-probes (especially the "period" parameter).
Since these checks will result in prematurely closed database connections, what do you think about extending the metadata exchange to indicate this is a health probe (and so don't need to initiate a connection to the db)?
There was a problem hiding this comment.
Removed the loop.
Fine with possibly integrating this with metadata exchange though that'll need some more thinking of how we go about that first...
nancynh
force-pushed
the
backend-check
branch
3 times, most recently
from
d395472 to
474e7ea
Compare
enocom
left a comment
enocom
left a comment
There was a problem hiding this comment.
Code looks good. I think we should double-check with our Cloud Run friends if we have the correct understanding of Direct VPC Egress setup times.
nancynh
force-pushed
the
backend-check
branch
2 times, most recently
from
79a1957 to
f539078
Compare
|
Discussed this a bit offline - moved the check to be in the startup probe instead in consideration of https://github.com/learnk8s/kubernetes-production-best-practices/blob/master/application-development.md#apps-are-independent. Gonna hold off on merging this PR though to see what the Direct VPC Egress folks say first |
enocom
left a comment
enocom
left a comment
There was a problem hiding this comment.
We might want to manually test this to confirm it works as intended. Otherwise LGTM.
3 participants
This PR adds an optional flag to set that will continuously check that a connection can be established to the server before returning a ready status in the health probe.
Fixes #909.